Replace Report.ExpandClassTimeline with [ClassTimeline] attribute by thomhurst · Pull Request #5875 · thomhurst/TUnit

thomhurst · 2026-05-10T17:09:05Z

Closes #5871.

Summary

Replaces the global TUnitSettings.Default.Report.ExpandClassTimeline (added in Show multi-step test spans in class timeline, align report ordering with execution, and correlate linked OTel activities #5847) with a per-class opt-in attribute [ClassTimeline(TimelineMode.FullExecution)].
Class-level usage wins over assembly-level via IScopedAttribute (same pattern as [NotInParallel]).
Net code change in the reporter: zero. The attribute writes a tunit.report.timeline custom property at discovery; existing TestExtensions.cs plumbing already flows that into ReportTestResult.CustomProperties. The JS reads it from group.tests[0].customProperties to gate the timeline branch.

Why per-class beats a global flag

Most projects mix BDD [DependsOn] chains (where the multi-step view is gold) with independent-test classes (where it just adds noise). The single global toggle from #5847 forced an all-or-nothing trade. Per-class lets BDD classes opt in without polluting the rest of the report.

API surface change

Removed:

TUnit.Core.Settings.ReportSettings
TUnit.Core.Settings.TUnitSettings.Report

Added:

TUnit.Core.ClassTimelineAttribute(TimelineMode) — [AttributeUsage(Class | Assembly)]
TUnit.Core.Enums.TimelineMode { Collapsed = 0, FullExecution = 1 }

ReportSettings shipped in #5847 with no downstream consumers, so this is a clean swap rather than a breaking change in practice. PublicAPI snapshots updated for net4.7/net8/net9/net10.

Test plan

TUnit.Engine.Tests/HtmlReporterTests (13/13 pass) — added two cases: attribute exposes Mode/ScopeType; tunit.report.timeline=FullExecution round-trips into the embedded report JSON via ReportTestResult.CustomProperties.
TUnit.UnitTests/TUnitSettingsTests (6/6 pass) — reverted the _savedExpandClassTimeline snapshot/restore additions from Show multi-step test spans in class timeline, align report ordering with execution, and correlate linked OTel activities #5847.
TUnit.PublicAPI Core net10 verified (snapshots updated for all four TFMs).
Full TUnit.slnx build clean (0 errors, pre-existing warnings only).
End-to-end smoke: drop [ClassTimeline(TimelineMode.FullExecution)] on a 3-test BDD chain in a sample project, run with the HTML reporter, confirm the class timeline shows test-case spans + child activities (and a sibling class without the attribute still shows only suite/init/dispose). Recommend the reviewer eyeballs this on real output before merge.

) The global setting from #5847 was too coarse for repos that mix BDD [DependsOn] chains with classes of independent tests. This swaps it for a per-class opt-in: [ClassTimeline(TimelineMode.FullExecution)] public class OrderProcessingFlow { ... } Class-level wins over assembly-level via IScopedAttribute. Attribute writes a custom property at discovery; the JS reads it from the group's first test — no new reporter code path, no global setting.

codacy-production · 2026-05-10T17:11:15Z

Not up to standards ⛔

🔴 Issues 1 critical

Alerts:
⚠ 1 issue (≤ 0 issues of at least minor severity)

Results:
1 new issue

Category Results

Security 1 critical

View in Codacy

🟢 Metrics 0 complexity

Metric Results

Complexity 0

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer}
_{TIP This summary will be updated as you push new changes.}

claude

Code Review — Replace `Report.ExpandClassTimeline` with `[ClassTimeline]` attribute

Summary

Clean, well-motivated migration. Per-class opt-in is the right design for this use case — a global toggle is genuinely wrong for mixed codebases (BDD chains + independent test classes side-by-side). The IScopedAttribute pattern is the established TUnit convention for this kind of attribute-priority question. Overall this is good work; a few things worth calling out below.

What's good

Design alignment — Using IScopedAttribute with ScopeType => typeof(ClassTimelineAttribute) mirrors [NotInParallel] exactly. Class-wins-over-assembly scoping comes for free from the runtime with no bespoke logic.
Zero data-pipeline change — The attribute pushes data via DiscoveredTestContext.AddProperty at discovery time; the existing TestExtensions.cs → ReportTestResult.CustomProperties flow carries it through untouched. No new coupling between the core attribute and the reporter.
Contract test is good — GenerateHtml_RoundTrips_ClassTimeline_CustomProperty_OnTest pins the C#/JS string contract ("tunit.report.timeline" / "FullExecution") at a level that will catch regressions if either side changes.
JS defensive coding — isClassTimelineFullExecution correctly handles group.tests being falsy/empty before calling .some(...), so no runtime exception on an empty group.

Issues / Suggestions

1. [ClassTimeline(TimelineMode.Collapsed)] writes a custom property that has no visible effect

// ClassTimelineAttribute.cs
public ValueTask OnTestDiscovered(DiscoveredTestContext context)
{
    context.AddProperty(ClassTimelinePropertyKey, Mode.ToString()); // always writes
    return default;
}

When Mode == Collapsed, the JS check (value === 'FullExecution') returns false, so the property is written but ignored. This is not a bug, but it is wasteful and potentially confusing: a user who explicitly writes [ClassTimeline(TimelineMode.Collapsed)] gets no change in behaviour over having no attribute at all.

Suggested fix: guard the write, or document why the no-op write is intentional:

public ValueTask OnTestDiscovered(DiscoveredTestContext context)
{
    if (Mode != TimelineMode.Collapsed)
        context.AddProperty(ClassTimelinePropertyKey, Mode.ToString());
    return default;
}

Alternatively, remove Collapsed from the public API entirely — or give it a concrete opt-out semantic so the class-level value can override an assembly-level FullExecution. That second interpretation is arguably more useful: [ClassTimeline(TimelineMode.Collapsed)] on a class inside an assembly tagged [assembly: ClassTimeline(TimelineMode.FullExecution)] would explicitly collapse just that class. The current JS only checks for FullExecution, so Collapsed already works as an override in practice, but the docs for Collapsed say "Same as the implicit default" rather than "Explicit opt-out when assembly is FullExecution" — the intent should be clearer.

2. No test covering assembly-level usage or class-wins-over-assembly scoping

The PR adds [AttributeUsage(Class | Assembly)] and mentions IScopedAttribute handles precedence, but neither ClassTimelineAttribute_Exposes_Mode_And_ScopeType nor any other new test exercises the assembly-level path or proves that a class-level attribute overrides an assembly-level one.

The runtime machinery for IScopedAttribute is covered elsewhere, but a simple test asserting the expected scoping behaviour would make the contract explicit and protect against a future regression where someone changes ScopeType:

[Test]
public void ClassTimeline_ClassLevelWins_OverAssemblyLevel()
{
    // IScopedAttribute contract: same ScopeType → class-level instance wins
    var asm   = new ClassTimelineAttribute(TimelineMode.FullExecution);
    var klass = new ClassTimelineAttribute(TimelineMode.Collapsed);
    asm.ScopeType.ShouldBe(klass.ScopeType); // same type → runtime de-dupes by scope
}

This is minor, but the PR description explicitly calls out this scoping as a key design decision.

3. ClassTimelinePropertyKey is internal but the roundtrip test accesses it directly

// HtmlReporterTests.cs
new ReportKeyValue { Key = ClassTimelineAttribute.ClassTimelinePropertyKey, ... }

This works today only because InternalsVisibleTo (or a shared test assembly) grants access. If the visibility is narrowed, the test breaks silently. The string literal "tunit.report.timeline" is also hardcoded in embedded.ShouldContain(...) two lines later — so the constant isn't being used consistently within the same test. Either use the constant throughout, or accept the literal everywhere and drop the internal const. Mixed usage is the worst of both.

Dual-mode check (CLAUDE.md requirement)

ITestDiscoveryEventReceiver.OnTestDiscovered is part of the core interfaces invoked by both the source-gen and reflection paths. No source-generator changes are needed here since the attribute carries its own discovery logic. This appears to satisfy the dual-mode requirement.

Verdict

Approve with the above noted. Issue #1 (the Collapsed no-op semantic) is the most worth addressing before merge because it affects public API clarity. Issues #2 and #3 are minor and can be addressed in a follow-up if preferred.

claude · 2026-05-10T17:12:45Z

Code Review — Replace `Report.ExpandClassTimeline` with `[ClassTimeline]` attribute

Summary

Clean, well-motivated migration. Per-class opt-in is the right design for this use case — a global toggle is genuinely wrong for mixed codebases (BDD chains + independent test classes side-by-side). The IScopedAttribute pattern is the established TUnit convention for this kind of attribute-priority question. Overall this is good work; a few things worth calling out below.

What's good

Design alignment — Using IScopedAttribute with ScopeType => typeof(ClassTimelineAttribute) mirrors [NotInParallel] exactly. Class-wins-over-assembly scoping comes for free from the runtime with no bespoke logic.
Zero data-pipeline change — The attribute pushes data via DiscoveredTestContext.AddProperty at discovery time; the existing TestExtensions.cs → ReportTestResult.CustomProperties flow carries it through untouched. No new coupling between the core attribute and the reporter.
Contract test is good — GenerateHtml_RoundTrips_ClassTimeline_CustomProperty_OnTest pins the C#/JS string contract ("tunit.report.timeline" / "FullExecution") at a level that will catch regressions if either side changes.
JS defensive coding — isClassTimelineFullExecution correctly handles group.tests being falsy/empty before calling .some(...), so no runtime exception on an empty group.

Issues / Suggestions

1. [ClassTimeline(TimelineMode.Collapsed)] writes a custom property that has no visible effect

// ClassTimelineAttribute.cs
public ValueTask OnTestDiscovered(DiscoveredTestContext context)
{
    context.AddProperty(ClassTimelinePropertyKey, Mode.ToString()); // always writes
    return default;
}

When Mode == Collapsed, the JS check (value === 'FullExecution') returns false, so the property is written but ignored. This is not a bug, but it is wasteful and potentially confusing: a user who explicitly writes [ClassTimeline(TimelineMode.Collapsed)] gets no change in behaviour over having no attribute at all.

Suggested fix — guard the write, or document why the no-op write is intentional:

public ValueTask OnTestDiscovered(DiscoveredTestContext context)
{
    if (Mode != TimelineMode.Collapsed)
        context.AddProperty(ClassTimelinePropertyKey, Mode.ToString());
    return default;
}

Alternatively, give Collapsed a concrete opt-out semantic: [ClassTimeline(TimelineMode.Collapsed)] on a class inside an assembly tagged [assembly: ClassTimeline(TimelineMode.FullExecution)] would explicitly collapse just that class. The current JS only checks for FullExecution, so Collapsed already works as an override in practice — but the docs say "Same as the implicit default" rather than "Explicit opt-out when assembly is FullExecution". The intent should be clearer either way.

2. No test covering assembly-level usage or class-wins-over-assembly scoping

The PR adds [AttributeUsage(Class | Assembly)] and mentions IScopedAttribute handles precedence, but neither new test exercises the assembly-level path or proves a class-level attribute overrides an assembly-level one. A simple assertion would make the contract explicit:

[Test]
public void ClassTimeline_ScopeType_Is_Same_For_All_Instances()
{
    var asm   = new ClassTimelineAttribute(TimelineMode.FullExecution);
    var klass = new ClassTimelineAttribute(TimelineMode.Collapsed);
    // same ScopeType → runtime de-dupes by scope (class wins)
    asm.ScopeType.ShouldBe(klass.ScopeType);
}

Minor, but the PR description calls this out as a key design decision.

3. ClassTimelinePropertyKey used inconsistently in the same test

// HtmlReporterTests.cs — line that uses the constant:
new ReportKeyValue { Key = ClassTimelineAttribute.ClassTimelinePropertyKey, Value = nameof(TimelineMode.FullExecution) }

// Lines that hardcode the same string two lines later:
embedded.ShouldContain("\"key\":\"tunit.report.timeline\"");
embedded.ShouldContain("\"value\":\"FullExecution\"");

Either use the constant throughout (including the ShouldContain assertions), or accept the literal everywhere and drop the internal const. Mixed usage means a rename of the key constant won't fully protect the test.

Dual-mode check (CLAUDE.md requirement)

ITestDiscoveryEventReceiver.OnTestDiscovered is part of the core interfaces invoked by both the source-gen and reflection paths. No source-generator changes are needed here since the attribute carries its own discovery logic. This appears to satisfy the dual-mode requirement.

Verdict

Good change overall — approve with the above noted. Issue #1 (the Collapsed no-op semantic) is the most worth addressing before merge since it affects public API clarity. Issues #2 and #3 are minor and can be handled in a follow-up if preferred.

Updated [TUnit.Core](https://github.com/thomhurst/TUnit) from 1.43.11 to 1.44.0. <details> <summary>Release notes</summary> _Sourced from [TUnit.Core's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.44.0  ## What's Changed ### Other Changes * Generated mocks live in the same namespace as the mocked type by @thomhurst in thomhurst/TUnit#5870 * Show multi-step test spans in class timeline, align report ordering with execution, and correlate linked OTel activities by @Copilot in thomhurst/TUnit#5847 * fix: don't leak RUC onto Should-style comparer overloads (#5857) by @thomhurst in thomhurst/TUnit#5873 * Fix culture-dependent timestamp in HTML test report (#5868) by @thomhurst in thomhurst/TUnit#5872 * fix(mocks-http): auto-prepend `/` to partial endpoint paths (#5838) by @thomhurst in thomhurst/TUnit#5874 * Replace Report.ExpandClassTimeline with [ClassTimeline] attribute by @thomhurst in thomhurst/TUnit#5875 * feat(assertions): make ShouldAssertion<T> implement IAssertion (#5824) by @thomhurst in thomhurst/TUnit#5876 * feat(mocks): support non-span ref struct out/ref params by @thomhurst in thomhurst/TUnit#5878 * fix(core): fill optional params when invoking MethodDataSource via reflection by @thomhurst in thomhurst/TUnit#5880 * Mocks: structural fix for Mock<T> / mocked-member name collisions by @thomhurst in thomhurst/TUnit#5881 * chore(mocks): promote TUnit.Mocks packages to stable by @thomhurst in thomhurst/TUnit#5877 ### Dependencies * chore(deps): update tunit to 1.43.41 by @thomhurst in thomhurst/TUnit#5863 * chore(deps): update dependency tunit.assertions.fsharp to 1.43.41 by @thomhurst in thomhurst/TUnit#5865 * chore(deps): bump @babel/plugin-transform-modules-systemjs from 7.28.5 to 7.29.4 in /docs by @dependabot[bot] in thomhurst/TUnit#5867 * chore(deps): bump fast-uri from 3.1.0 to 3.1.2 in /docs by @dependabot[bot] in thomhurst/TUnit#5862 **Full Changelog**: thomhurst/TUnit@v1.43.41...v1.44.0 ## 1.43.41  ## What's Changed ### Other Changes * feat(playwright): expose default Context/Launch options on settings by @thomhurst in thomhurst/TUnit#5861 * fix(hooks): resolve TestDiscovery hook context type by attribute kind, not method name by @thomhurst in thomhurst/TUnit#5860 ### Dependencies * chore(deps): update tunit to 1.43.38 by @thomhurst in thomhurst/TUnit#5858 **Full Changelog**: thomhurst/TUnit@v1.43.38...v1.43.41 ## 1.43.38  ## What's Changed ### Other Changes * feat(playwright): add TUnitPlaywrightSettings defaults by @thomhurst in thomhurst/TUnit#5859 **Full Changelog**: thomhurst/TUnit@v1.43.37...v1.43.38 ## 1.43.37  ## What's Changed ### Other Changes * docs: clarify MethodDataSourceAttribute.Factory is source-generator-managed by @Copilot in thomhurst/TUnit#5835 * fix(assertions): skip ref-struct members in IsEquivalentTo (#5841) by @thomhurst in thomhurst/TUnit#5842 * feat(playwright): add composition-based fixtures by @thomhurst in thomhurst/TUnit#5840 ### Dependencies * chore(deps): update tunit to 1.43.11 by @thomhurst in thomhurst/TUnit#5821 * chore(deps): update dependency polyfill to 10.4.0 by @thomhurst in thomhurst/TUnit#5830 * chore(deps): update dependency polyfill to 10.4.0 by @thomhurst in thomhurst/TUnit#5829 * chore(deps): update react to ^19.2.6 by @thomhurst in thomhurst/TUnit#5839 * chore(deps): update dependency polyfill to 10.5.0 by @thomhurst in thomhurst/TUnit#5848 * chore(deps): update dependency polyfill to 10.5.0 by @thomhurst in thomhurst/TUnit#5849 * chore(deps): update aspire to 13.3.0 by @thomhurst in thomhurst/TUnit#5851 * chore(deps): update dependency brace-expansion to v5.0.6 by @thomhurst in thomhurst/TUnit#5853 * chore(deps): update dependency polyfill to 10.5.1 by @thomhurst in thomhurst/TUnit#5854 * chore(deps): update dependency polyfill to 10.5.1 by @thomhurst in thomhurst/TUnit#5855 * chore(deps): update verify to 31.16.3 by @thomhurst in thomhurst/TUnit#5856 **Full Changelog**: thomhurst/TUnit@v1.43.11...v1.43.37 Commits viewable in [compare view](thomhurst/TUnit@v1.43.11...v1.44.0). </details> [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=TUnit.Core&package-manager=nuget&previous-version=1.43.11&new-version=1.44.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

thomhurst temporarily deployed to Pull Requests May 10, 2026 17:09 — with GitHub Actions Inactive

thomhurst had a problem deploying to Pull Requests May 10, 2026 17:09 — with GitHub Actions Failure

claude Bot reviewed May 10, 2026

View reviewed changes

thomhurst enabled auto-merge (squash) May 10, 2026 17:13

thomhurst merged commit eca2a0a into main May 10, 2026
12 of 14 checks passed

thomhurst deleted the feat/class-timeline-attribute-5871 branch May 10, 2026 17:37

dependabot Bot mentioned this pull request May 11, 2026

Bump TUnit from 1.43.11 to 1.44.0 shouldly/shouldly#1198

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Replace Report.ExpandClassTimeline with [ClassTimeline] attribute#5875

Replace Report.ExpandClassTimeline with [ClassTimeline] attribute#5875
thomhurst merged 1 commit into
mainfrom
feat/class-timeline-attribute-5871

thomhurst commented May 10, 2026

Uh oh!

codacy-production Bot commented May 10, 2026

Uh oh!

claude Bot left a comment

Uh oh!

claude Bot commented May 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

thomhurst commented May 10, 2026

Summary

Why per-class beats a global flag

API surface change

Test plan

Uh oh!

codacy-production Bot commented May 10, 2026

Not up to standards ⛔

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Code Review — Replace Report.ExpandClassTimeline with [ClassTimeline] attribute

Summary

What's good

Issues / Suggestions

Dual-mode check (CLAUDE.md requirement)

Verdict

Uh oh!

claude Bot commented May 10, 2026

Code Review — Replace Report.ExpandClassTimeline with [ClassTimeline] attribute

Summary

What's good

Issues / Suggestions

Dual-mode check (CLAUDE.md requirement)

Verdict

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Code Review — Replace `Report.ExpandClassTimeline` with `[ClassTimeline]` attribute

Code Review — Replace `Report.ExpandClassTimeline` with `[ClassTimeline]` attribute